chore(lint): enable clippy::expect_used#98
Merged
Conversation
`.expect()` aborts the process with no chance to recover or attach
context, unlike the `anyhow::Result` propagation this crate already
uses throughout its command layer. Fix the seven production call
sites: three `strip_prefix(...).expect("caller checked ...")`
invariants in `issue/parse/*` (already excluded from the coverage
gate) become `unreachable!()` instead of a panic-macro detour, since
`clippy::panic` is already denied here; two `dirs::home_dir()`
lookups now propagate via `anyhow::Context` in functions that already
return `Result`; and the remaining two (a path-building helper with
no `Result` to propagate into, and a true invariant in `cmd_restore`
that would need a dead, coverage-breaking branch to express any other
way) keep their documented panic behind a localized, reasoned
`#[allow]`. Test-only `.expect()` calls (fixtures, where a
descriptive panic message is the idiomatic way to fail fast) are
exempted via a single `cfg(test)` crate attribute and an integration
test file allow.
Closes #97
Member
Author
|
cc @tupe12334 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Enables
clippy::expect_usedin[lints.clippy]and fixes the violations it surfaces.issue/parse/{centy,gh,gitlab}.rs:strip_prefix(...).expect("caller checked ...")→let Some(..) = .. else { unreachable!(..) }(these files are already excluded from the coverage gate;unreachable!avoids apanic!detour sinceclippy::panicis already denied here).multi_workspace.rs::github_bare_path: now returnsResult<PathBuf>and propagates a missing home directory viaanyhow::Contextinstead of aborting (single call site, already inside aResult-returning caller).commands/restore.rs::cmd_restore: samedirs::home_dir()fix viaanyhow::Context.issue/paths.rs::bare_clone_path_rootedandcommands/restore.rs'spath.parent()call: both are true invariants with noResultto propagate into (the former is a documented, intentional panic on the publicbare_clone_pathAPI; the latter would need a structurally-dead branch that breaks the repo's 100%-line-coverage pre-push gate). Both keep their.expect()behind a localized, reasoned#[allow(clippy::expect_used)]..expect()calls (fixtures, where a descriptive panic message is the idiomatic way to fail a test fast) are exempted via#![cfg_attr(test, allow(clippy::expect_used))]inlib.rsand a matching file-level allow intests/snapshot_tests.rs.Closes #97
Test plan
cargo buildcargo clippy --all-targets --all-features -- -D warningscargo test(253 + 30 + 43 + 22 + 6 tests, all passing)cargo fmt --checkThis pull request was opened by the Clippy lint improvements → issue + PR (Rust repos) → Slack routine of moadim.